CachingAllocator: report the current requested size in reused block - #51730
CachingAllocator: report the current requested size in reused block#51730felicepantaleo wants to merge 1 commit into
Conversation
…ck is reused tryReuseCachedBlock assigned the cached block descriptor over the descriptor of the new allocation, and then restored the queue. That also replaced block.requested with the size requested by the previous allocation that used the block, so cacheStatus().requested reported the sizes of earlier allocations. Take from the cached block only what the cache owns, that is the buffer, the event and the bin size, and leave the queue, the bin and the requested size of the new allocation in place. This also removes one of the two copies of the block descriptor, and resolves the two open TODOs. Take ownership of the cached block with extract before it is modified, and update the accounting after the operations that can throw. Add a test that allocates two different sizes from the same bin and checks the reported sizes.
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51730/50657
|
|
A new Pull Request was created by @felicepantaleo for master. It involves the following packages:
@cmsbuild, @fwyzard, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
type bugfix |
|
type ngt |
|
-1 Failed Tests: RelVals-AMD_W7900 DAS Queries: The DAS query tests failed, see the summary page for details. Failed RelVals-AMD_W7900
Comparison SummarySummary:
NVIDIA_H100 Comparison SummarySummary:
NVIDIA_L4 Comparison SummarySummary:
NVIDIA_L40S Comparison SummarySummary:
NVIDIA_T4 Comparison SummarySummary:
Max Memory Comparisons exceeding threshold NVIDIA_H100@cms-sw/core-l2 , I found 1 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
Max Memory Comparisons exceeding threshold NVIDIA_L40S@cms-sw/core-l2 , I found 1 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
What dos it mean ? |
|
I have no idea what the description of the PR even means. Please make only the change that is necessary for the fix, and write a reasonable and correct description. |
|
-heterogeneous |
What operations do you think can throw ? |
|
Can you do a minimal fix, for example // associate the cached buffer to the new queue
auto queue = std::move(*(block.queue));
+ // update the requested size
+ auto requested = block.requested;
// TODO cache (or remove) the debug information and use std::move()
block = iBlock->second;
block.queue = std::move(queue);
+ block.requested = requested;? |
|
#51741 has (only) the minimal fix. |
3 participants
When
tryReuseCachedBlockreuses a cached block, it copies the whole cached descriptor over the descriptor of the new allocation. This also overwritesblock.requestedwith the size requested by the previous user of the block.cacheStatus().requestedtherefore reports stale sizes. The accounting does not drift, becausefree()subtracts the same value thatallocate()added, but the reported value is wrong after any reuse.